Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Add switch types to set animation switching effects #228

Merged
merged 1 commit into from
Aug 2, 2024
Merged

feat: Add switch types to set animation switching effects #228

merged 1 commit into from
Aug 2, 2024

Conversation

starhcq
Copy link
Contributor

@starhcq starhcq commented Aug 2, 2024

Add switch types to set animation switching effects

Log: Add switch types to set animation switching effects
Bug: https://pms.uniontech.com/bug-view-168549.html

@pengfeixx
Copy link
Contributor

[是否满足兼容性要求] Y
[是否满足commit提交规范] Y
[是否满足编码规范] Y
[Review结论] Pass
[Fail原因] N/A

@starhcq
feat: Add switch types to set animation switching effects
7386f56 
   Add switch types to set animation switching effects

Log: Add switch types to set animation switching effects
Bug: https://pms.uniontech.com/bug-view-168549.html
@deepin-ci-robot
Copy link

deepin pr auto review

关键摘要:

  • 在AlbumTitle.qml中，新增的blocksignal属性可能是一个错误，因为它在updateIndex函数中被设置，但在onCurrentIndexChanged信号处理中并没有使用。
  • 在StatusBar.qml中，修改了状态栏的显示逻辑，但未提供上下文说明为什么要这样做。
  • 在ThumbnailImage.qml中，移除了setCollecttionViewIndex函数，但没有提供替代逻辑或说明为什么移除。
  • 在globalstatus.cpp和globalstatus.h中，新增了currentSwitchType的读写属性，但没有提供相应的文档注释。
  • 在types.h中，新增了SwitchType枚举，但没有提供相应的单元测试来验证枚举的正确性。

是否建议立即修改:
是

  • 应该立即修复AlbumTitle.qml中的blocksignal属性的问题，确保它被正确使用。
  • 需要提供更多的上下文和文档注释来解释为什么在StatusBar.qml中进行了这些更改，以及这些更改的影响。
  • 应该确认移除setCollecttionViewIndex函数是否是必要的，并补充相应的替代逻辑。
  • 对于新增的currentSwitchType属性，应该添加相应的文档注释来说明其用途和行为。
  • 应该为新增的SwitchType枚举添加单元测试来验证其正确性。

rb-union
rb-union reviewed Aug 2, 2024
View reviewed changes
Copy link
Contributor

@rb-union rb-union left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[是否满足兼容性要求] Y
[是否满足commit提交规范] Y
[是否满足编码规范] Y
[Review结论] Pass
[Fail原因] N/A

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pengfeixx, starhcq

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@starhcq starhcq merged commit 2f8eed3 into linuxdeepin:master Aug 2, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rb-union rb-union rb-union left review comments

@pengfeixx pengfeixx pengfeixx approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@starhcq @pengfeixx @deepin-ci-robot @rb-union